Skip to content

Conversation

@Endilll
Copy link
Contributor

@Endilll Endilll commented Jan 6, 2025

One of our contributors got confused by the behavior of the script when they incorrectly specified the status of a CWG issue (https://github.com/llvm/llvm-project/pull/102857/files#r1904264872), and this is an attempt to reduce the confusion:

  • script doesn't write anything to disk unless everything is good;
  • error messages are prefixed with (a very familiar) error: to make it more obvious that a fatal error occurred.

@Endilll Endilll added the clang Clang issues not falling into any other category label Jan 6, 2025
@Endilll Endilll requested review from cor3ntin and zyn0217 January 6, 2025 16:04
@llvmbot
Copy link
Member

llvmbot commented Jan 6, 2025

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

One of our contributors got confused by the behavior of the script when they incorrectly specified the status of a CWG issue (https://github.com/llvm/llvm-project/pull/102857/files#r1904264872), and this is an attempt to reduce the confusion:

  • script doesn't write anything to disk unless everything is good;
  • error messages are prefixed with (a very familiar) error: to make it more obvious that a fatal error occurred.

Full diff: https://github.com/llvm/llvm-project/pull/121785.diff

1 Files Affected:

  • (modified) clang/www/make_cxx_dr_status (+11-9)
diff --git a/clang/www/make_cxx_dr_status b/clang/www/make_cxx_dr_status
index e0885fdbd2d3c2..05de96119fc18a 100755
--- a/clang/www/make_cxx_dr_status
+++ b/clang/www/make_cxx_dr_status
@@ -83,8 +83,8 @@ else:
 
 status_map = collect_tests()
 drs = get_issues(issue_list_path)
-out_file = open(output, 'w')
-out_file.write('''\
+out_html = []
+out_html.append('''\
 <!DOCTYPE html>
 <!-- This file is auto-generated by make_cxx_dr_status. Do not modify. -->
 <html>
@@ -149,7 +149,7 @@ def availability(issue):
     unresolved_status = unresolved_status_match.group(1)
     proposed_resolution_match = re.search(r' (open|drafting|review|tentatively ready|ready) (\d{4}-\d{2}(?:-\d{2})?|P\d{4}R\d+)$', status)
     if proposed_resolution_match is None:
-      raise AvailabilityError('Issue {}: \'{}\' status should be followed by a paper number (P1234R5) or proposed resolution in YYYY-MM-DD format'.format(dr.issue, unresolved_status))
+      raise AvailabilityError('error: issue {}: \'{}\' status should be followed by a paper number (P1234R5) or proposed resolution in YYYY-MM-DD format'.format(dr.issue, unresolved_status))
     proposed_resolution = proposed_resolution_match.group(2)
     status = status[:-1-len(proposed_resolution)]
     status = status[:-1-len(unresolved_status)]
@@ -236,7 +236,7 @@ def availability(issue):
     avail = 'Duplicate of <a href="#%s">%s</a>' % (dup, dup)
     _, avail_style, _, _ = availability(dup)
   else:
-    raise AvailabilityError('Unknown status %s for issue %s' % (status, dr.issue))
+    raise AvailabilityError('error: unknown status %s for issue %s' % (status, dr.issue))
   return (avail + avail_suffix, avail_style, unresolved_status, details)
 
 count = {}
@@ -266,7 +266,7 @@ for dr in drs:
     else:
       if unresolved_status != dr.status:
         availability_error_occurred = True
-        print("Issue %s is marked '%s', which differs from CWG index status '%s'" \
+        print("error: issue %s is marked '%s', which differs from CWG index status '%s'" \
              % (dr.issue, unresolved_status, dr.status))
         continue
   else:
@@ -280,7 +280,7 @@ for dr in drs:
 
     if unresolved_status:
       availability_error_occurred = True
-      print("Issue %s is marked '%s', even though it is resolved in CWG index" \
+      print("error: issue %s is marked '%s', even though it is resolved in CWG index" \
            % (dr.issue, unresolved_status))
       continue
 
@@ -296,7 +296,7 @@ for dr in drs:
         <summary>{avail}</summary>
         {details}
       </details>'''
-  out_file.write(f'''
+  out_html.append(f'''
   <tr{row_style} id="{dr.issue}">
     <td><a href="https://cplusplus.github.io/CWG/issues/{dr.issue}.html">{dr.issue}</a></td>
     <td>{dr.status}</td>
@@ -310,12 +310,14 @@ if availability_error_occurred:
 for status, num in sorted(count.items()):
   print("%s: %s" % (status, num), file=sys.stderr)
 
-out_file.write('''\
+out_html.append('''\
 </table>
 
 </div>
 </body>
 </html>
 ''')
-out_file.close()
 
+out_file = open(output, 'w')
+out_file.write(''.join(out_html))
+out_file.close()

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the patch looks great

@Endilll Endilll merged commit cf23549 into llvm:main Jan 7, 2025
10 checks passed
@Endilll Endilll deleted the build-cxx-dr-status-in-memory branch January 7, 2025 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang Clang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants